feat(gateway): safely rotate ACME credentials - #935
Merged
Conversation
kvinwang
force-pushed
the
codex/feat-gateway-exact-sni-certificates
branch
from
August 4, 2026 15:25
13cde75 to
f6190fd
Compare
kvinwang
force-pushed
the
codex/fix-gateway-corrupt-acme-credentials
branch
from
August 4, 2026 15:25
d2c19d6 to
2e4c354
Compare
kvinwang
force-pushed
the
codex/feat-gateway-exact-sni-certificates
branch
from
August 5, 2026 05:10
f6190fd to
48f65e3
Compare
kvinwang
force-pushed
the
codex/fix-gateway-corrupt-acme-credentials
branch
from
August 5, 2026 05:10
4649a10 to
823e39a
Compare
kvinwang
force-pushed
the
codex/fix-gateway-corrupt-acme-credentials
branch
from
August 5, 2026 05:45
823e39a to
888e360
Compare
kvinwang
changed the base branch from
codex/feat-gateway-exact-sni-certificates
to
master
August 5, 2026 05:48
Rework RotateAcmeCredentials so a partial failure no longer strands the cluster on an unpublished account: - validate every domain's DNS credential before registering the replacement account, so a misconfigured domain aborts with no side effects and no rate-limited ACME registration consumed - publish the new credentials before re-pinning CAA; a partial re-pin now converges by rerunning SetCaa instead of registering yet another account on every retry - re-pin CAA best-effort across all domains and name the failed ones, with the recovery path, in the returned error - serialize rotation across nodes with a best-effort TTL lock in WaveKV; without CAS it narrows the race window rather than guaranteeing mutual exclusion, and the comments say so - refuse to auto-register a fresh account on ACME URL mismatch, which left every domain's CAA pinned to the old account; point at RotateAcmeCredentials, which re-pins CAA along with the switch - derive AcmeInfo.account_uri from the published credentials so the dashboard reflects a rotation even when the best-effort attestation write is skipped; attach the quote only when it matches that account - add the rotation step to the staging-to-production tutorial Verification: - cargo test -p dstack-gateway: 74 passed (7 in distributed_certbot) - cargo clippy -p dstack-gateway --all-features -- -D warnings --allow unused_variables: clean
This was referenced Aug 5, 2026
…ocking Address review findings on the rotation rework: - get_acme_credentials now distinguishes a missing or deleted key from a record that no longer decodes; corruption at the WaveKV encoding layer previously read as absence and silently registered a fresh ACME account that the account-bound CAA records refuse. Issuance and AcmeInfo now fail with a pointer at RotateAcmeCredentials instead. - move account attestation after CAA re-pinning: its agent round trips do not gate issuance and must not widen the window where the published account and the CAA records disagree; still run it on partial re-pin failure so the new account is recorded - release the rotation lock only when the visible value is the one this rotation wrote, so a holder that outlived the timeout cannot delete the lock of the node that took over - re-pin the first domain through the registration client instead of constructing a second DNS client and re-fetching the ACME directory; this also removes an expect() that failed CI's panic lints (-D clippy::expect_used) Verification: - cargo test -p dstack-gateway: 77 passed (new: corrupt-record fail-closed at the KV layer, tombstone vs corruption, stale-holder lock release guard) - cargo clippy -- -D warnings -D clippy::expect_used -D clippy::unwrap_used --allow unused_variables: clean (workspace) - cargo fmt --all -- --check: clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A corrupt persisted ACME credential must not take down Gateway or discard certificates that are already serving traffic. Operators also need a controlled way to replace the shared ACME account and its account-bound CAA records — including the documented staging→production directory switch, where the previous behavior silently registered a fresh account while every domain's CAA stayed pinned to the old one, blocking all issuance.
Solution
Fail closed on corrupt credentials — at both layers. A stored credential record that no longer decodes (WaveKV MessagePack layer) or whose embedded JSON is malformed fails the affected issuance/renewal attempt with an error pointing at the rotation RPC, without affecting Gateway startup, TLS serving, or other certificate state.
get_acme_credentialsdistinguishes a missing/deleted key (None) from corruption (Err); treating corruption as absence would silently register a fresh account that the account-bound CAA records refuse. The previous silent re-registration was not a real self-heal for the same reason.Admin-only
RotateAcmeCredentialsRPC, sequenced so that failures are convergent:issue/issuewildCAA records for every ZT domain, best effort across all domains; the first domain reuses the registration client. Failed domains are named in the returned error together with the recovery path.Publishing before re-pinning makes partial failure recoverable without registering another account: rerun
SetCaa(idempotent) until it succeeds. With the reverse order, every retry would burn one of Let's Encrypt's 10 new registrations per IP per 3 hours and the already-re-pinned domains would forbid the still-published old account.Cross-node serialization via a best-effort TTL lock in WaveKV, alongside the in-process guard shared with CAA reconciliation. WaveKV has no CAS, so the lock narrows the concurrent-rotation window to replication latency rather than guaranteeing mutual exclusion; operators should still rotate through one gateway at a time. Release is guarded: a rotation that outlived the timeout does not delete the lock of the node that took over.
ACME URL mismatch now refuses to auto-register and points at
RotateAcmeCredentials, which re-pins CAA along with the switch. The gateway setup tutorial's staging→production step now includes the rotation call.AcmeInfo.account_uriderives from the published credentials instead of the best-effort attestation record, so the dashboard reflects a rotation even when quote generation is unavailable; the attestation quote is attached only when it matches the current account.Private ACME credential material is never returned through the API.
Known limitations
SetCaarun (gateway: no automatic CAA reconciliation loop #1010).;guard records; a failure can leave a domain blocked from issuance until aSetCaarerun succeeds (same pre-existing hazard asSetCaaitself, documented on both paths).Verification
cargo test -p dstack-gateway— 77 passed (distributed_certbot: rotation lock/precondition cases, stale-holder release guard, URL matching, CAA reconciliation guards;kv::acme_credentials_tests: corrupt-record fail-closed, tombstone vs corruption)cargo clippy -- -D warnings -D clippy::expect_used -D clippy::unwrap_used --allow unused_variables— clean (workspace, CI flags)cargo fmt --all -- --check,git diff --check